-
Notifications
You must be signed in to change notification settings - Fork 218
Update observed gen on no-update #697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… into update-observed-gen-on-noupdate # Conflicts: # operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ReconciliationDispatcher.java
} | ||
|
||
private void updateStatusObservedGenerationIfRequired(R resource) { | ||
// todo: change this to check for HasStatus (or similar) when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't like comments in general, but actually though about it and the HasStatus resources would make sense to us only if the standard kuberenetes resources would also implement observed generation aware too. What is quite a big change again in fabric8 client. Also not sure if makes sense for a controller which handles the standard kubernetes objects side by side with it's standard controller to manage observed generations. So IMHO we should support this only for custom resources. But wanted to discuss this with the fabtic8 team too, maybe today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. That said, the todo
comment shows up in IDEA so it's interesting to keep these until we're sure we've addressed what they're referring to, generally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.